Skip to content

fix(web): stop modals closing on backdrop click - #326

Open
claude[bot] wants to merge 3 commits into
mainfrom
fix/credential-modal-backdrop-dismiss
Open

fix(web): stop modals closing on backdrop click#326
claude[bot] wants to merge 3 commits into
mainfrom
fix/credential-modal-backdrop-dismiss

Conversation

@claude

@claude claude Bot commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Requested by Jake Hulberg · Slack thread

Summary

Clicking the dimmed backdrop outside a modal was closing it, discarding any in-progress work. This makes backdrop-dismiss off by default for every modal in the app, broadening the earlier credential-only fix per reviewer request.

Before: clicking the dimmed backdrop outside ANY modal closed it, discarding in-progress work (credential setup, service setup, etc.).

After: an outside/backdrop click no longer closes any modal. Modals close via the X button, Cancel, or Escape. This applies app-wide.

How: flipped the shared web/src/components/Modal.tsx dismissOnBackdrop default to false (the opt-in prop is retained for any modal that wants backdrop-dismiss back). The always-rendered header X button plus per-modal Cancel/Escape guarantee every modal stays closeable. Dropdown menus are unaffected — they use a separate pattern and intentionally keep outside-click-to-close.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • CI / build

Test plan

  • Existing tests pass (make test) — not run; change is frontend-only
  • Added/updated tests for new behavior
  • Manual testing (describe below)

Frontend build verified with npm ci && npm run build (tsc && vite build) — compiles cleanly. Manual check: clicking the backdrop no longer closes any modal; the X, Cancel, and Escape still close them.

Security checklist

  • No secrets or credentials in code
  • No new unauthenticated endpoints
  • Input validation on new API surfaces
  • Checked for OWASP top 10 (injection, XSS, etc.)

Generated by Claude Code

claude added 2 commits July 20, 2026 02:37
@claude
fix(web): keep credential setup modal open on backdrop click
db86d19 
An accidental click on the dimmed backdrop no longer discards
in-progress credential setup. Backdrop dismissal is now opt-in via a
new dismissOnBackdrop prop (default true) on the shared Modal, and the
credential modal opts out.
@claude
fix(web): disable backdrop-click dismissal for all modals
13556f2 
Backdrop-click-to-close is now off by default across every modal
(Escape and the X/Cancel buttons still close them), broadening the
earlier credential-only fix per reviewer request.
@claude claude Bot changed the title fix(web): keep credential setup modal open on backdrop click fix(web): stop modals closing on backdrop click Jul 20, 2026
@jakehulberg
jakehulberg marked this pull request as ready for review July 20, 2026 02:46
@greptile-apps

greptile-apps Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR changes the shared Modal component so backdrop (outside-area) clicks no longer close modals by default, preventing accidental data loss when forms or multi-step flows are open. The Escape key and the always-visible X button remain valid close mechanisms.

  • dismissOnBackdrop prop is added with a default of false; the outer wrapper's onClick is now onClose only when the prop is true, preserving opt-in backdrop dismissal for any modal that wants it.
  • The existing e.stopPropagation() on the card div correctly guards against click-through in the opt-in case; no regression is introduced.
  • All callers silently gain the new default — no call-site changes are needed, and no modal loses its X-button close path.

Confidence Score: 5/5

Safe to merge — single-line default change in a shared presentational component with no data or auth logic involved.

The change is a one-prop addition with a changed default on a purely visual component. The existing stopPropagation guard on the card keeps opt-in backdrop dismissal correct, Escape-key close is preserved as documented, and no call sites need updating. No logic, data, or security paths are touched.

No files require special attention.

Important Files Changed

Filename Overview
web/src/components/Modal.tsx Adds opt-in dismissOnBackdrop prop (default false), changing the outer wrapper's onClick from always-onClose to conditionally undefined. The existing stopPropagation on the card keeps the opt-in path safe; Escape-key close is intentionally preserved.

Reviews (1): Last reviewed commit: "fix(web): disable backdrop-click dismiss..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@saifsmailbox98 saifsmailbox98 saifsmailbox98 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@saifsmailbox98 @claude